Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(error message): Clarify magic number mismatch error message between heap child information #118

Open
wants to merge 1 commit into
base: msys2-3_3_6-release
Choose a base branch
from

Conversation

Quenty
Copy link

@Quenty Quenty commented Nov 23, 2022

msys2 is a fork of cygwin but the error message for magic_versions is cygwin specific. Removing this message may remove a lot of documentation for users searching online.

However, the documentation itself is not useful. Users will not find a cygwin version nor does the help online specify exact information. While cygwin was intended to be installed at a single location, msys2 is used in a variety of stand alone tools, one of which is git-for-windows. Whenever a version mismatch can occur this can lead to programs such as VS Code breaking other git installations, or vice-versa. In this regard a bit of a better error message can help people figure out what is wrong on their msys2 or cygwin installed codebases.

Docs PR:
msys2/msys2.github.io#235

@Quenty
Copy link
Author

Quenty commented Nov 23, 2022

Sorry if this PR doesn't follow guidelines or should be submitted in another way. I couldn't find any documentation on this approach. Feel free to ignore if this isn't super useful, this is a tricky topic.

Copy link
Member

@jeremyd2019 jeremyd2019 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure the added text really clarifies anything. The idea of searching and destroying alternate cygwin1.dll wouldn't help as the names are different with msys2, and the idea of doing that with msys-2.0.dll is also problematic because git-for-windows' version comes from a different fork, and anyway just deleting dlls is likely to break things worse. The best advice is just to not mix these environments, but I don't know how to word a message to that effect that would make sense.

@@ -1360,7 +1360,10 @@ multiple_cygwin_problem (const char *what, uintptr_t magic_version, uintptr_t ve
system_printf ("%s magic number mismatch detected - %p/%ly", what, magic_version, version);
else
api_fatal ("%s mismatch detected - %ly/%ly.\n\
This problem is probably due to using incompatible versions of the cygwin DLL.\n\
This problem is probably due to using incompatible versions of the msys2 or cygwin DLL.\n\
msys2 is uses pinned memory to share child information. All versions running must be the same magic_version.\n\
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was probably supposed to be either is using or just uses.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch!

winsup/cygwin/dcrt0.cc Show resolved Hide resolved
winsup/cygwin/dcrt0.cc Outdated Show resolved Hide resolved
msys2 is a fork of cygwin but the error message for magic_versions is cygwin specific. Removing this message may remove a lot of documentation for users searching online.

However, the documentation itself is not useful. Users will not find a cygwin version nor does the help online specify exact information. While cygwin was intended to be installed at a single location, msys2 is used in a variety of stand alone tools, one of which is git-for-windows. Whenever a version mismatch can occur this can lead to programs such as VS Code breaking other git installations, or vice-versa. In this regard a bit of a better error message can help people figure out what is wrong on thier msys2 or cygwin installed codebases.
@Quenty
Copy link
Author

Quenty commented Dec 13, 2022

Ok, I've updated this with more links as well as more verbose documentation about what cygwin recommends, versus what msys2 runtime may support.

@Quenty Quenty force-pushed the users/quenty/fix_msys_error_message branch from 28eb9ef to a548536 Compare December 14, 2022 00:33
@dscho
Copy link
Collaborator

dscho commented Dec 14, 2022

@lazka how do we want to proceed with regards to v3.4.2?

@lazka
Copy link
Member

lazka commented Dec 14, 2022

@lazka how do we want to proceed with regards to v3.4.2?

I've been testing it for the last few days and I'm not aware of any issues. I'll prepare a PR. edit: msys2/MSYS2-packages#3409

@lazka
Copy link
Member

lazka commented Dec 15, 2022

@lazka how do we want to proceed with regards to v3.4.2?

we found another regression: https://cygwin.com/pipermail/cygwin/2022-December/252699.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants